-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[GlobalISel] Allow some load/store instructions to be folded in Match Table backend #70830
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
… Table backend Load/store instruction can be folded into non-adjacent instruction within the same basic block, if there are no other instructions with memory/other side effects between them.
|
@llvm/pr-subscribers-llvm-globalisel Author: Vladislav Dzhidzhoev (dzhidzhoev) ChangesLoad/store instruction can be folded into non-adjacent instruction Full diff: https://github.com/llvm/llvm-project/pull/70830.diff 1 Files Affected:
diff --git a/llvm/lib/CodeGen/GlobalISel/GIMatchTableExecutor.cpp b/llvm/lib/CodeGen/GlobalISel/GIMatchTableExecutor.cpp
index 26752369a7711a1..3441df8a342e4f1 100644
--- a/llvm/lib/CodeGen/GlobalISel/GIMatchTableExecutor.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/GIMatchTableExecutor.cpp
@@ -70,6 +70,25 @@ bool GIMatchTableExecutor::isObviouslySafeToFold(MachineInstr &MI,
if (MI.isConvergent() && MI.getParent() != IntoMI.getParent())
return false;
- return !MI.mayLoadOrStore() && !MI.mayRaiseFPException() &&
- !MI.hasUnmodeledSideEffects() && MI.implicit_operands().empty();
+ auto IsSafe = [](const MachineInstr &MI) {
+ return !MI.mayRaiseFPException() && !MI.hasUnmodeledSideEffects() &&
+ MI.implicit_operands().empty();
+ };
+ auto IsSafeNoMem = [IsSafe](const MachineInstr &MI) {
+ return !MI.mayLoadOrStore() && IsSafe(MI);
+ };
+
+ // If source instruction uses memory, fold if no intermediate
+ // instructions use it.
+ if (MI.mayLoadOrStore() && IsSafe(MI) &&
+ MI.getParent() == IntoMI.getParent()) {
+ auto IntoIt = IntoMI.getIterator();
+ auto NextIt = std::next(MI.getIterator());
+ while (!NextIt.isEnd() && NextIt != IntoIt && IsSafeNoMem(*NextIt))
+ ++NextIt;
+ if (NextIt == IntoIt)
+ return true;
+ }
+
+ return IsSafeNoMem(MI);
}
|
arsenm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs tests
| !MI.hasUnmodeledSideEffects() && MI.implicit_operands().empty(); | ||
| auto IsSafe = [](const MachineInstr &MI) { | ||
| return !MI.mayRaiseFPException() && !MI.hasUnmodeledSideEffects() && | ||
| MI.implicit_operands().empty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't understand the implicit operands check. I think this is starting to reproduce MachineInstr::isSafeToMove
|
reverse ping? |
arsenm
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be reinventing isSafeToMove
|
Implemented in #101675. |
Load/store instruction can be folded into non-adjacent instruction
within the same basic block, if there are no other instructions with
memory/other side effects between them.